-
Notifications
You must be signed in to change notification settings - Fork 264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fit remote code execution in history and template imports functionality #1625
base: master
Are you sure you want to change the base?
Conversation
if (!isset($_POST['token'])) { | ||
error_log('WARNING: A POST request detected with no csrf token found'); | ||
die('Authentication failed.'); | ||
} else if (!hash_equals(hash_hmac('sha256', '/letter.php.theform', $_SESSION['token']), $_POST['token'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is verifyCsrfTokenAndCompareHash
not used here rather?
if (!isset($_POST['token'])) { | ||
error_log('WARNING: A POST request detected with no csrf token found'); | ||
die('Authentication failed.'); | ||
} else if (!(CsrfToken::verifyCsrfToken($_POST['token'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're missing a closing parenthesis here too
// Function to verify a csrf token using with second token | ||
function verifyCsrfTokenAndCompareHash($token, $secondToken) | ||
{ | ||
if (hash_equals(hash_hmac('sha256', $secondToken, $_SESSION['token']), $token) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're missing a closing parenthesis here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its necessary to type cast the inputs before supplying to hash_hmac, interesting things might occur if that is not done ( type juggling vulnerability ), if $secondToken is a user controlled input, if an array is passed, hash_hmac will return null.
For more details:
https://www.youtube.com/watch?v=MpeaSNERwQA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @naveen17797 I will add that too.
if (!isset($_POST['token'])) { | ||
error_log('WARNING: A POST request detected with no csrf token found'); | ||
die('Authentication failed.'); | ||
} else if (!(CsrfToken::verifyCsrfToken($_POST['token'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're missing a closing parenthesis here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @maggienegm. I am guessing you fixed in here PR #1634?? have not checked yet. If you did then I can just close this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes @Ngai-E , I fixed the closing parenthesis issues there
@Ngai-E can you rebase thanks |
No description provided.